Changes error code to when test is undefined#91
Changes error code to when test is undefined#91deiga wants to merge 4 commits intonpm:latestfrom deiga:changes-npm-test-exit-code-to-1
Conversation
|
This is a pretty impactful change, and not to be taken lightly. Why do you think it's worth the potential negative impact on thousands of users? |
|
@zkat I am well aware that this will have a rather large impact. In my oppinion there is a fairly large discrepancy in how this script works on a UX level. Most UNIX commands return 1 if something is not defined, just like Let's think of building a (rather silly) CI pipeline example, where I know that all projects will always be I think it makes a lot of sense to uniform the Interface of how Someone in the |
|
My $0.02 on this: I think it's a great idea, and long overdue. I don't love the implementation, though. I think you can just delete the default Like, I seem to recall that I made it do that once upon a time, probably about 7 years ago or so, and some folks objected because they had setups that ran As a compromise, I made TL;DRMy suggestion:
|
|
@isaacs I really like your suggestion of just deleting the special handling, it makes the most sense here. |
|
This will be included in npm v7. It is a breaking change, or I'd get it done sooner, but it looks good to me. Thanks for your continued patience. |
|
@isaacs I fixed the merge conflicts. Can I do something else to prepare this? |
|
This is looking good. Thanks! It is a breaking change, but I am sure it can get into npm 7. |
|
@deiga Hey! Sorry for the delay, we've essentially refactored this code in |
Changes the CLI to be more in line with how Unix operates.
This is in line with how
npm fooandnpm run foobehave andnpm initgenerates a tests script which hasexit 1defined.Moved from npm/npm#21026
This change is